Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: Teach the CLI to ignore empty commands #30265

Merged
merged 2 commits into from
May 1, 2018

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Apr 30, 2018

Cause the CLI to ignore commands that are empty or consist only of
newlines. This is a fairly standard thing for SQL CLIs to do.

It looks like:

sql> ;
sql>
   |
   | ;
sql> exit;
Bye!

I think I could have implemented this with a CliCommand that throws
out empty string but it felt simpler to bake it in to the CliRepl.

Closes #30000

Cause the CLI to ignore commands that are empty or consist only of
newlines. This is a fairly standard thing for SQL CLIs to do.

It looks like:
```
sql> ;
sql>
   |
   | ;
sql> exit;
Bye!
```

I think I *could* have implemented this with a `CliCommand` that throws
out empty string but it felt simpler to bake it in to the `CliRepl`.

Closes elastic#30000
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@nik9000 nik9000 added >bug and removed >enhancement labels Apr 30, 2018
@nik9000
Copy link
Member Author

nik9000 commented Apr 30, 2018

I'm not sure if I consider this a "bug fix" or an "enhancement". It'd be nice to backport it to 6.3 though.

@nik9000 nik9000 added >enhancement and removed >bug labels Apr 30, 2018
@nik9000
Copy link
Member Author

nik9000 commented Apr 30, 2018

It'd be nice to backport it to 6.3 though.

We'll leave it for 6.4 so 6.3 can stay as stable as possible.

@costin
Copy link
Member

costin commented Apr 30, 2018

LGTM.

@nik9000
Copy link
Member Author

nik9000 commented Apr 30, 2018

Thanks for reviewing @costin! I'll merge once we get a successful packaging test run on master.

@nik9000 nik9000 merged commit abe797b into elastic:master May 1, 2018
nik9000 added a commit that referenced this pull request May 1, 2018
Cause the CLI to ignore commands that are empty or consist only of
newlines. This is a fairly standard thing for SQL CLIs to do.

It looks like:
```
sql> ;
sql>
   |
   | ;
sql> exit;
Bye!
```

I think I *could* have implemented this with a `CliCommand` that throws
out empty string but it felt simpler to bake it in to the `CliRepl`.

Closes #30000
dnhatn added a commit that referenced this pull request May 2, 2018
* master: (68 commits)
  [DOCS] Removes X-Pack Elasticsearch release notes (#30272)
  Correct an example in the top-level suggester documentation. (#30224)
  [DOCS] Removes broken link
  [DOCS] Adds file realm configuration details (#30221)
  [DOCS] Adds PKI realm configuration details (#30225)
  Fix a reference to match_phrase_prefix in the match query docs. (#30282)
  Fix failure for validate API on a terms query (#29483)
  [DOCS] Fix 6.4-specific link in changelog (#30314)
  Remove RepositoriesMetaData variadic constructor (#29569)
  Test: increase authentication logging for debugging
  [DOCS] Removes redundant SAML realm settings (#30196)
  REST Client: Add Request object flavored methods (#29623)
  [DOCS] Adds changelog to Elasticsearch Reference (#30271)
  [DOCS] Fixes section error
  SQL: Teach the CLI to ignore empty commands (#30265)
  [DOCS] Adds Active Directory realm configuration details (#30223)
  [DOCS] Removes redundant file realm settings (#30192)
  [DOCS] Fixes users command name (#30275)
  Build: Move gradle wrapper jar to a dot dir (#30146)
  Build: Log a warning if disabling reindex-from-old (#30304)
dnhatn added a commit that referenced this pull request May 2, 2018
* 6.x: (62 commits)
  [DOCS] Adds new installation package details (#29590)
  Revert "Build: Move gradle wrapper jar to a dot dir (#30146)"
  [DOCS] Added 6.3.0 section to changelog
  [DOCS] Merge 6.x release notes into changelog (#30312)
  [DOCS] Removes broken link
  [DOCS] Adds file realm configuration details (#30221)
  [DOCS] Adds PKI realm configuration details (#30225)
  [DOCS] Fix 6.4-specific link in changelog (#30314)
  Remove RepositoriesMetaData variadic constructor (#29569)
  [DOCS] Adds changelog to Elasticsearch Reference (#30310)
  Test: increase authentication logging for debugging
  [DOCS] Removes redundant SAML realm settings (#30196)
  SQL: Teach the CLI to ignore empty commands (#30265)
  [DOCS] Fixes section error
  [DOCS] Adds Active Directory realm configuration details (#30223)
  [DOCS] Removes redundant file realm settings (#30192)
  [DOCS] Fixes users command name (#30275)
  Build: Move gradle wrapper jar to a dot dir (#30146)
  Build: Log a warning if disabling reindex-from-old (#30304)
  TEST: Add debug log to FlushIT
  ...
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants